Skip to content

fix(engine): include fairnessAnalytics in parseFocusManifest's emptiness check - #8414

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
philluiz2323:fix/focus-manifest-fairness-analytics-emptiness
Jul 24, 2026
Merged

fix(engine): include fairnessAnalytics in parseFocusManifest's emptiness check#8414
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
philluiz2323:fix/focus-manifest-fairness-analytics-emptiness

Conversation

@philluiz2323

Copy link
Copy Markdown
Contributor

Summary

parseFocusManifest's aggregate condition (deciding whether to emit a "no recognized focus fields" warning and force manifest.present = false) checks !manifest.X.present for every other optional block (gate, publicStats, draftFlow, ... federatedIntelligence), but omitted fairnessAnalytics even though it's parsed and assigned identically to every sibling block.

Concrete failure: a .loopover.yml containing only a populated fairnessAnalytics: block (e.g. fairnessAnalytics: { enabled: true }) was correctly parsed into manifest.fairnessAnalytics = { present: true, enabled: true }, but the aggregate check never saw it, so the manifest was still incorrectly forced to present: false with a misleading "no recognized focus fields" warning.

Fix

Added the missing !manifest.fairnessAnalytics.present term, in the same relative position fairnessAnalytics is parsed in (between publicStats and draftFlow). No other block's condition or parseFairnessAnalyticsConfig itself was touched.

Added a dedicated fairnessAnalytics: test block mirroring the existing publicStats: suite exactly -- this block had zero test coverage before this PR (confirmed: no existing test referenced parseFairnessAnalyticsConfig or the fairnessAnalytics field at all beyond an unrelated settings-alias fixture), including the specific regression case this issue describes.

Scope

Fixes #8366

Validation

  • git diff --check
  • npm run actionlint (no workflow changes)
  • npm run typecheck (clean)
  • npx vitest run test/unit/focus-manifest.test.ts -- 792/792 passing (9 new tests)
  • Coverage verified via --coverage.include="packages/loopover-engine/src/focus-manifest.ts": the new line is not in the uncovered-lines list; all pre-existing uncovered lines are unrelated to this diff
  • npm run docs:drift-check -- passes (102 FocusManifest fields all documented, unchanged field count since this adds a check, not a new field)
  • npm run ui:openapi:check (no API/schema changes)
  • npm audit --audit-level=moderate (no dependency changes)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why: no workflow, API, OpenAPI, or dependency surface is touched -- this is a one-line condition fix plus its own dedicated test block in packages/loopover-engine.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A -- no such changes.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A -- no API/OpenAPI/MCP changes.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A -- no UI changes.)
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots. (N/A -- zero apps/loopover-ui/** or any visual-surface changes in this PR.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

…ess check

parseFocusManifest's aggregate condition deciding whether to emit a "no
recognized focus fields" warning and force manifest.present = false
checks !manifest.X.present for every other optional block (gate,
publicStats, draftFlow, ... federatedIntelligence), but omitted
fairnessAnalytics even though it's parsed and assigned identically to
every sibling. A .loopover.yml containing only a populated
fairnessAnalytics: block was correctly parsed into
{ present: true, enabled: true } but then incorrectly forced back to
manifest.present = false with a misleading empty-manifest warning,
since the aggregate check never saw it.

Added the missing !manifest.fairnessAnalytics.present term in the same
position the field is parsed in (between publicStats and draftFlow).
Added a dedicated fairnessAnalytics: test block mirroring the existing
publicStats: suite -- this block had no test coverage at all before,
including the specific regression case (fairnessAnalytics-only manifest
stays present with no spurious warning).

Fixes JSONbored#8366
@philluiz2323
philluiz2323 requested a review from JSONbored as a code owner July 24, 2026 12:17
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.42%. Comparing base (712836f) to head (00a6c0f).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8414   +/-   ##
=======================================
  Coverage   92.42%   92.42%           
=======================================
  Files         791      791           
  Lines       79259    79259           
  Branches    23948    23947    -1     
=======================================
  Hits        73252    73252           
+ Misses       4868     4866    -2     
- Partials     1139     1141    +2     
Flag Coverage Δ
shard-1 53.29% <ø> (-0.04%) ⬇️
shard-2 52.44% <ø> (ø)
shard-3 56.16% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-engine/src/focus-manifest.ts 98.74% <ø> (+0.29%) ⬆️

... and 1 file with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 24, 2026
@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-24 12:45:55 UTC

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a minimal, correct one-line fix: `parseFocusManifest`'s aggregate emptiness check was missing `!manifest.fairnessAnalytics.present`, so a manifest containing only a populated `fairnessAnalytics:` block was incorrectly forced to `present: false` with a spurious warning. The fix is inserted in the exact right position (between `publicStats` and `draftFlow`, matching parse order), and the accompanying test block thoroughly covers the default/null/malformed/enabled/disabled/round-trip cases, directly exercising the regression scenario described in the issue. `focus-manifest-validation.ts`'s `focusManifestToNormalizedJson` doesn't call `fairnessAnalyticsConfigToJson` at all (pre-existing gap, not touched by this diff) so it's out of scope here.

Nits — 2 non-blocking
  • The `focusManifestToNormalizedJson` helper in `focus-manifest-validation.ts` still omits `fairnessAnalyticsConfigToJson` entirely (a pre-existing gap unrelated to this diff, but worth a follow-up issue since it means fairnessAnalytics never appears in normalized validation output).
  • Consider filing a follow-up issue for `focus-manifest-validation.ts`'s `focusManifestToNormalizedJson` to include `fairnessAnalyticsConfigToJson(manifest.fairnessAnalytics)`, mirroring every sibling block, since it's silently absent from validated/normalized manifest output today.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8366
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 1039 registered-repo PR(s), 613 merged, 126 issue(s).
Contributor context ✅ Confirmed Gittensor contributor philluiz2323; Gittensor profile; 1039 PR(s), 126 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff adds `!manifest.fairnessAnalytics.present` to the aggregate emptiness condition exactly as requested, and includes a new test asserting a manifest with only a populated fairnessAnalytics block sets present:true with no spurious warning, matching both deliverables.

Review context
  • Author: philluiz2323
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 1039 PR(s), 126 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 66cbefa into JSONbored:main Jul 24, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parseFocusManifest's emptiness check omits fairnessAnalytics, misreporting present:false

1 participant